Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move newline to inside the DisableAutoGenTag block #2030

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dramich
Copy link

@dramich dramich commented Sep 20, 2023

Fixes #2029

@CLAassistant
Copy link

CLAassistant commented Sep 20, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the area/docs-generation Generation of docs via Cobra label Sep 20, 2023
Copy link
Collaborator

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine. Is this more stylistic than anything? I'm surprised there are tools that have an opinion on markdown formatting. Or is it more that some version control tools don't want extra newlines at the end of files?

I'd like to understand more on why this is needed.

doc/md_docs.go Show resolved Hide resolved
@dramich
Copy link
Author

dramich commented May 18, 2024

There are tools that care about whitespace regardless of the file type, as stated in the original issue pre-commit has a end-of-file-fixer which looks for things like double returns. https://github.com/pre-commit/pre-commit-hooks?tab=readme-ov-file#end-of-file-fixer

The fix ensures the generated files only have a single return at the end of the file, it does not remove all of them.

If this is deemed not needed feel free to close it or let me know and I can.

@nirs
Copy link
Contributor

nirs commented May 18, 2024

It would be nice to add a little test to keep the output correct after this change.

@dramich
Copy link
Author

dramich commented May 18, 2024

Added a new assertion to the tests
Test fails before the change with:

go test github.com/spf13/cobra/doc
--- FAIL: TestGenMdNoTag (0.00s)
    cmd_test.go:110: Expected to not have suffix:
         "\n\n"
        Got: "## root\n\nRoot short description\n\n### Synopsis\n\nRoot long description\n\n```\nroot [flags]\n```\n\n### Options\n\n```\n  -h, --help              help for root\n  -r, --rootflag string    (default \"two\")\n  -t, --strtwo string     help message for parent flag strtwo (default \"two\")\n```\n\n### SEE ALSO\n\n* [root echo](root_echo.md)\t - Echo anything to the screen\n\n"

After fix the test passes. All other tests already pass as they don't have the double line break.

@@ -40,6 +42,7 @@ func TestGenMdDoc(t *testing.T) {
checkStringContains(t, output, echoSubCmd.Short)
checkStringOmits(t, output, deprecatedCmd.Short)
checkStringContains(t, output, "Options inherited from parent commands")
checkStringOmitsSuffix(t, output, doubleLineBreak)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice, but don't we want the more general check, that the output always ends with a single newline?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs-generation Generation of docs via Cobra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GenMarkdownCustom adds extra newline when DisableAutoGenTag is true
4 participants